-
Notifications
You must be signed in to change notification settings - Fork 974
remove webSecurity:false in webPreferences #10242
Conversation
js/stores/appStore.js
Outdated
sharedWorker: true, | ||
nodeIntegration: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is not needed anymore, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yea, does nothing
app/browser/tabs.js
Outdated
@@ -706,6 +706,7 @@ const api = { | |||
}, | |||
|
|||
executeScriptInBackground: (script, cb) => { | |||
// Do not edit without security review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is not a web-accessible api and is only used in the browser process. Actually now that we have worker threads it isn't used at all
js/stores/appStore.js
Outdated
partition: 'default', | ||
webSecurity: false, | ||
allowFileAccessFromFileUrls: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if these are necessary anymore either since we're using chrome://brave
urls now
@bridiver ready for review |
app/browser/tabs.js
Outdated
@@ -699,22 +698,6 @@ const api = { | |||
}) | |||
}, | |||
|
|||
executeScriptInBackground: (script, cb) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is being used in a PR in progress
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes this is used in this PR #10325
and add some comments for security-sensitive code fix #10240
4992f99
to
019f85b
Compare
fixed the merge conflict. @NejcZdovc @bridiver could one of you review plz |
no-qa-needed? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me
remove webSecurity:false in webPreferences
and add some comments for security-sensitive code
fix #10240
Submitter Checklist:
git rebase -i
to squash commits (if needed).Test Plan:
Reviewer Checklist:
Tests